Skip to content

butane: report CEX kernel arg error once when both sources set - #2294

Open
deepak0x wants to merge 1 commit into
coreos:mainfrom
deepak0x:fix/2285-dup-cex-error
Open

butane: report CEX kernel arg error once when both sources set#2294
deepak0x wants to merge 1 commit into
coreos:mainfrom
deepak0x:fix/2285-dup-cex-error

Conversation

@deepak0x

@deepak0x deepak0x commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

Config.Validate in butane/config/openshift/v4_23_exp/validate.go reported ErrMissingKernelArgumentCex twice at $.openshift.kernel_arguments when both boot_device.luks.cex.enabled: true and a storage.luks root entry with cex.enabled: true were set.

Two independent if-blocks each called r.AddOnError(...) when the rd.luks.key=/etc/luks/cex.key kernel argument was absent, so both fired and the identical error was reported twice.

Fix

Consolidate the two checks into a single cex flag (set when either the boot-device CEX is enabled or any root storage.luks entry has CEX enabled) and emit the error exactly once when the flag is set and the kernel argument is missing. Behavior and error path are unchanged.

Test plan

  • Added a TestValidateConfig case that sets both CEX sources and an empty kernel argument list, asserting exactly one ErrMissingKernelArgumentCex at $.openshift.kernel_arguments.
  • Verified the new test fails on the old code (two entries) and passes on the fixed code (one entry).
  • go test ./butane/config/openshift/v4_23_exp/... passes.

Fixes #2285

This PR ports the fix originally opened as coreos/butane#732 for coreos/butane#731, before butane was merged into ignition. It supersedes that PR and closes coreos/butane#731.

cc @prestist

@deepak0x
butane: report CEX kernel arg error once when both sources set
82f95e5 
When both boot_device.luks.cex.enabled and a storage.luks root entry
with cex.enabled are set, the missing 'rd.luks.key=/etc/luks/cex.key'
kernel argument was reported twice at $.openshift.kernel_arguments.

Consolidate the two independent checks into a single CEX-enabled flag
and emit the error exactly once. Behavior and error path are unchanged.

Fixes coreos#2285

Signed-off-by: Deepak Bhagat <deepak988088@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 24930c23-c228-4ac2-8313-c681bca7e696

📥 Commits

Reviewing files that changed from the base of the PR and between 5300eed and 82f95e5.

📒 Files selected for processing (3)
  • butane/config/openshift/v4_23_exp/validate.go
  • butane/config/openshift/v4_23_exp/validate_test.go
  • docs/release-notes.md

Included review availability: Your plan includes up to 4 reviews per rolling hour; 1 remains after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Include the required Apache 2.0 license header at the top of every Go source file.
Use the project's import ordering in Go files: standard library imports, blank line, project packages, blank line, then external dependencies.
Follow the project's Go naming conventions: exported identifiers use PascalCase, unexported identifiers use camelCase, and filenames use snake_case.

Files:

  • butane/config/openshift/v4_23_exp/validate_test.go
  • butane/config/openshift/v4_23_exp/validate.go
docs/**

⚙️ CodeRabbit configuration file

docs/**: Documentation served via GitHub Pages/Jekyll. Every platform must be documented in supported-platforms.md. The ./test script validates doc consistency.

Files:

  • docs/release-notes.md
🔇 Additional comments (3)
butane/config/openshift/v4_23_exp/validate.go (1)

55-57: LGTM!

butane/config/openshift/v4_23_exp/validate_test.go (1)

233-269: LGTM!

docs/release-notes.md (1)

22-23: LGTM!

📝 Walkthrough

Walkthrough

OpenShift 4.23 validation now combines boot-device and root-storage CEX settings before checking the required kernel argument. A regression test verifies one error, and the release notes document the fix.

Changes

CEX validation

Layer / File(s) Summary
Combine CEX validation state
butane/config/openshift/v4_23_exp/validate.go		 CEX state includes the boot-device setting before storage entries are scanned. The missing kernel-argument check runs once after both sources are evaluated.
Regression coverage and release note
butane/config/openshift/v4_23_exp/validate_test.go, docs/release-notes.md		 A combined CEX configuration test expects exactly one missing kernel-argument error. The release notes describe the fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 82f95

This change reports the missing CEX kernel argument error once instead of twice when both supported configuration sources are enabled, with no change to validation behavior or error location. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation consolidates both CEX sources into one validation check and adds a regression test, satisfying issue #2285.
Out of Scope Changes check ✅ Passed The code change, regression test, and release note directly support the linked issue and contain no unrelated changes.
Commit Message Convention ✅ Passed The PR has one non-merge commit: butane: report CEX kernel arg error once when both sources set; report is lowercase imperative and has no trailing period.
Title check ✅ Passed The title uses the required subsystem prefix, imperative wording, lowercase description, and no trailing period; it accurately describes the change.
Description check ✅ Passed The description clearly explains the duplicate CEX validation error, the fix, the test coverage, and the related issue.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@deepak0x deepak0x mentioned this pull request Aug 18, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

1 participant

@deepak0x